Skip to content

api: Fix pagination for list PublicIPAddresses#5231

Merged
yadvr merged 2 commits intoapache:4.15from
shapeblue:fix-publicip-pagination
Jul 27, 2021
Merged

api: Fix pagination for list PublicIPAddresses#5231
yadvr merged 2 commits intoapache:4.15from
shapeblue:fix-publicip-pagination

Conversation

@Pearl1594
Copy link
Contributor

Description

Fixes: #5229
This PR fixes the issue noticed while listing public IP addresses wrt pagination. The count is set to the number of items being returned as opposed to the total number of entities (here, public IPs)

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

via cmk:

Prior Fix:

## Total number of ip addresses returned without pagination
list publicipaddresses  listall=true filter=id
{
  "count": 55,
  "publicipaddress": [
    {
       ....
    }
    ]
}    

## With Pagination

 list publicipaddresses  pagesize=5 page=1 listall=true filter=id
{
  "count": 5,    <--- while count has to be 55, it returns 5 which is the value of pagesize
  "publicipaddress": [
    {
      "id": "152f39af-1af7-4413-bbf1-1132b1708a9d"
    },
    {
      "id": "cbd3f921-53b5-45a6-9a1b-8c5877e9be34"
    },
    {
      "id": "425204aa-8efd-400a-bc07-98a14fae00b3"
    },
    {
      "id": "dc5b206d-db46-4ab5-9ede-da63261648e5"
    },
    {
      "id": "437fb07f-2c9e-4583-805b-710474c045be"
    }
  ]
}

Post Fix:

## Total number of addresses without pagination:
SBCM5> > list publicipaddresses listall=true filter=id
{
  "count": 20,
  "publicipaddress": [
    {
      ....
    }
  ]
}
  
## With Pagination:  
SBCM5> > list publicipaddresses listall=true filter=id, page=1 pagesize=5
{
  "count": 20,  <--- Returns the actual number of public IP addresses
  "publicipaddress": [
    {
      "id": "a000effc-0dea-4a84-8299-2de2e38b6d79"
    },
    {
      "id": "c0d594a4-bca2-43c2-bbb2-32ab9eea13a4"
    },
    {
      "id": "d270125d-9578-4d56-9453-96a80c00e721"
    },
    {
      "id": "8fe27d41-ab1a-41e2-8e24-0c78ab389603"
    },
    {
      "id": "04466b5d-b61f-4ff2-9d41-c4354966acf4"
    }
  ]
}

@Pearl1594
Copy link
Contributor Author

@blueorangutan package

@blueorangutan
Copy link

@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@Pearl1594 Pearl1594 requested a review from shwstppr July 22, 2021 03:40
@blueorangutan
Copy link

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 626

@Pearl1594
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@Pearl1594 Pearl1594 linked an issue Jul 22, 2021 that may be closed by this pull request
Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested for different cases, correct items and count returned by API.

(localcloud) 🐦 > list publicipaddresses listall=true page=1 pagesize=2
{
  "count": 3,
...
(localcloud) 🐧 > list publicipaddresses listall=true page=1 pagesize=2 ipaddress=192.168.2.3 
{
  "count": 1,
  "publicipaddress": [
...
(localcloud) 🦍 > list publicipaddresses listall=true page=1 pagesize=2 allocatedonly=true 
{
  "count": 3,
...
(localcloud) 🦍 > list publicipaddresses listall=true page=1 pagesize=15 allocatedonly=false filter=id
{
  "count": 395,
...

@Pearl1594
Copy link
Contributor Author

@blueorangutan package

@Pearl1594 Pearl1594 added this to the 4.15.2.0 milestone Jul 22, 2021
@blueorangutan
Copy link

@Pearl1594 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 628

@Pearl1594
Copy link
Contributor Author

@blueorangutan test

@blueorangutan
Copy link

@Pearl1594 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

Trillian test result (tid-1351)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41427 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5231-t1351-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_deploy_virtio_scsi_vm.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_iptables_default_policy.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_router_nics.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Smoke tests completed. 87 look OK, 0 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File

Copy link
Contributor

@nvazquez nvazquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM

@nvazquez nvazquez closed this Jul 26, 2021
@nvazquez nvazquez reopened this Jul 26, 2021
@nvazquez
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@nvazquez a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian. SL-JID 653

@nvazquez
Copy link
Contributor

@blueorangutan test

@blueorangutan
Copy link

@nvazquez a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

@blueorangutan
Copy link

Trillian test result (tid-1379)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 46327 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr5231-t1379-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_routers_network_ops.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_vpn.py
Intermittent failure detected: /marvin/tests/smoke/test_hostha_kvm.py
Smoke tests completed. 86 look OK, 1 have error(s)
Only failed tests results shown below:

Test Result Time (s) Test File
test_01_create_redundant_VPC_2tiers_4VMs_4IPs_4PF_ACL Failure 495.30 test_vpc_redundant.py
test_03_create_redundant_VPC_1tier_2VMs_2IPs_2PF_ACL_reboot_routers Failure 423.73 test_vpc_redundant.py
test_05_rvpc_multi_tiers Failure 471.86 test_vpc_redundant.py

@yadvr yadvr merged commit 826e479 into apache:4.15 Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Public IP Addresses return 20 records even if there are more records

5 participants